Skip to content

Refactor date range picker#1225

Merged
david-crespo merged 5 commits into
mainfrom
sliding-interval-fix
Oct 15, 2022
Merged

Refactor date range picker#1225
david-crespo merged 5 commits into
mainfrom
sliding-interval-fix

Conversation

@david-crespo

@david-crespo david-crespo commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator

This change makes it easier to slot in a date picker lib and it makes the whole thing work better.

  • Use classic local state + onChange instead of Formik for tracking input states
  • Accordingly, use plain UI <TextInput> and <Listbox> instead of Formik-ized <TextField> and <ListboxField> for the inputs
    • This also lets us not show the ugly headers, instead using aria-label directly for SR users
  • Instead of a big hook returning { startTime, endTime, pickerElement }, we now have a normal component and a much smaller hook managing startTime and endTime, plus setting them from the initial value of the preset
  • This ended up being a small part of it, but I also fixed an issue where the picker values wouldn't move as the window slides

The net line increase is not real; it's all in yarn.lock. I upgraded vitest while trying to debug a timer issue in the tests and left it upgraded. Line count without that (git diff --stat main.. -- . ":(exclude)yarn.lock") is +387 -383

Before

2022-10-13-date-range-picker-before

After

2022-10-13-date-range-picker-after

@vercel

vercel Bot commented Oct 12, 2022

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
console-ui-storybook ✅ Ready (Inspect) Visit Preview Oct 13, 2022 at 9:39PM (UTC)

@david-crespo david-crespo changed the title Date range picker uses local state instead of Formik Refactor date range picker Oct 13, 2022

expect(onChange).toBeCalledWith(start, now)

vi.useRealTimers()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weirdly, when I had the vi.setSystemTime in a beforeAll or a beforeEach, it did not work. Something to remember, possibly a bug to report.

@david-crespo david-crespo merged commit 7f2f8a9 into main Oct 15, 2022
@david-crespo david-crespo deleted the sliding-interval-fix branch October 15, 2022 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant